Honor AUTH_ROLE_PUBLIC in FastAPI API server#65685
Honor AUTH_ROLE_PUBLIC in FastAPI API server#65685gaurav0107 wants to merge 10 commits intoapache:mainfrom
Conversation
The FastAPI API server's bearer-token auth raised 401 for anonymous users even when AUTH_ROLE_PUBLIC was set, because the gate only honored Flask's webserver_config.py and the FastAPI stack had no equivalent. This adds a new [fab] auth_role_public Airflow config, surfaced via a new BaseAuthManager.get_public_user() hook. FabAuthManager populates an AnonymousUser with the public role's permissions pre-expanded so it works outside a Flask request context. The legacy AUTH_ROLE_PUBLIC setting in webserver_config.py keeps working via a startup-time bridge that copies [fab] auth_role_public into Flask's config. Closes apache#60897 Co-Authored-By: Claude <noreply@anthropic.com>
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
Per @vincbeck's review, replace the BaseAuthManager.get_public_user() method with a middleware approach mirroring SimpleAllAdminMiddleware. - Add generic BaseAuthManager.get_fastapi_middlewares() extension hook that auth managers override to register their own middlewares. - New FabAuthRolePublicMiddleware attaches an AnonymousUser to request.state.user for unauthenticated requests when [fab] auth_role_public is configured. The FastAPI get_user dependency short-circuits on request.state.user, so no JWT minting is needed. - Revert security.py to upstream behavior (401 on missing token). - Rename FabAuthManager.get_public_user -> build_public_user and mark as :meta private: since it's an implementation detail consumed only by the middleware. Addresses apache#65685 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@gaurav0107 This PR has been converted to draft because it does not yet meet our Pull Request quality criteria. Issues found:
What to do next:
Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack. |
…iring Follow-up review from @vincbeck on PR apache#65685. Shrinks the diff and aligns the FAB public-access path with the simple auth manager's patterns. - ``FabAuthManager.build_public_user`` no longer opens a Flask app context or uses ``self.flask_app``. It resolves the public role via a SQLAlchemy session (``@provide_session``), eager-loaded joined permissions, and writes ``_roles``/``_perms`` directly on the ``AnonymousUser``. No Flask context is created in the auth manager. - ``FabAuthManager._get_auth_role_public`` reads only the Flask app config. The ``providers/fab/www/app.py`` bridge already copies ``[fab] auth_role_public`` into ``AUTH_ROLE_PUBLIC`` at app creation, so there's a single source of truth and no duplicate ``conf.get``. - ``SimpleAuthManager`` now overrides ``get_fastapi_middlewares`` to register ``SimpleAllAdminMiddleware`` when ``[core] simple_auth_manager_all_admins`` is set. The hardcoded registration in ``init_middlewares`` is removed so all auth managers go through the same generic hook. - Revert the ``webserver-authentication.rst`` expansion; the existing ``AUTH_ROLE_PUBLIC`` paragraph is enough and the new config key is documented via ``provider.yaml``. - Drop the 65685 newsfragment per maintainer feedback. Tests updated to match the new behavior (Flask config is the single source of truth read by the auth manager) and extended with two new tests for ``SimpleAuthManager.get_fastapi_middlewares``. Co-Authored-By: Claude <noreply@anthropic.com>
|
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item: Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — please mark the thread as resolved yourself by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project. |
vincbeck
left a comment
There was a problem hiding this comment.
One nit, otherwise, looks good :)
Per @vincbeck's review nit: airflow-core documentation should not mention a specific provider auth manager. Rephrase the docstring example to describe the behavior generically (public access) rather than naming FAB. Addresses apache#65685 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CI is failing |
CI mypy caught two issues on the middleware refactor: 1. core_api/app.py:174 — ``add_middleware`` expects ``_MiddlewareFactory[[KwArg(Any)]]`` not bare ``type``. Narrow ``get_fastapi_middlewares`` return type on BaseAuthManager (and the FAB/simple overrides) to use Starlette's ``_MiddlewareFactory`` protocol. 2. FAB middleware.py:44 — ``get_auth_manager()`` is typed ``BaseAuthManager[Any]``, so mypy cannot resolve ``build_public_user`` (FAB-specific). Cast to ``FabAuthManager`` inside ``dispatch`` since this middleware is only registered by FabAuthManager. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
``FabAuthManager._get_auth_role_public`` previously read only ``self.appbuilder.app.config["AUTH_ROLE_PUBLIC"]``, which requires an active Flask application context. ``get_fastapi_middlewares`` is invoked during FastAPI startup by ``init_middlewares``, before any Flask context exists, so the werkzeug ``LocalProxy`` behind ``appbuilder.app`` raised ``RuntimeError: Working outside of application context`` and crashed the api-server container (surfaced in PROD Docker Compose quick start, Kubernetes, e2e, remote logging, and XCom object storage jobs). Read ``[fab] auth_role_public`` from the Airflow config first so startup works without a Flask context, and fall back to the Flask config when an app context is available — that still honors the legacy ``AUTH_ROLE_PUBLIC`` entry in ``webserver_config.py``. A narrow ``RuntimeError`` guard around the Flask access keeps the code safe even if another caller reaches it before a context is pushed. Also regenerate ``get_provider_info.py`` to include the new ``auth_role_public`` entry so the ``update-providers-build-files`` prek hook is satisfied.
Summary
Closes #60897.
The FastAPI API server's auth gate rejected anonymous users with 401 even when
AUTH_ROLE_PUBLICwas set inwebserver_config.py, because the gate only consulted Flask's config. The Flask legacy webserver honored it; FastAPI did not.Approach
Per @jason810496's proposal (endorsed by @vincbeck in the issue thread):
[fab] auth_role_publicAirflow config — the canonical source going forward.BaseAuthManager.get_public_user()hook (returnsNoneby default).FabAuthManager.get_public_user()resolves the configured role, builds anAnonymousUserwith_roles/_permspre-populated insideapp_context(), and returns it.resolve_user_from_token()in the FastAPI security layer consultsauth_manager.get_public_user()before raising 401.providers/fab/www/app.pycopies[fab] auth_role_publicinto Flask'sAUTH_ROLE_PUBLICso the legacy webserver's existing code paths stay in sync — no behavior change for users onwebserver_config.pyonly.Why the
AnonymousUser._rolesdirect-write?FAB's
AnonymousUser.rolesis a lazy property that callssecurity_manager.get_public_role()on every access, which requires a Flask request context.get_public_user()runs under FastAPI where there is no request context, so we build the user inside a briefapp_context(), resolve permissions eagerly, and write them directly to_roles/_permsto freeze the snapshot. This is the minimum surface needed to avoid touching FAB's property implementation while keeping the call safe outside Flask.Type of change
Testing
Backwards compatibility
Users relying on `AUTH_ROLE_PUBLIC = "Admin"` in `webserver_config.py` continue to work unchanged — the Flask webserver keeps consulting that value, and the FastAPI gate now also honors it via the startup bridge.
cc @vincbeck @jason810496
🤖 Generated with Claude Code